[MRG] change utils.logger.warning -> utils.warn #9434
Conversation
|
CIs were failing when I pushed the changes for the in particular, this function was the problem: Lines 29 to 41 in f42f612 and it looks like the scipy PR for this temporary fix was merged: scipy/scipy#12676. So perhaps it could be removed now (not sure, because the numpy issue related to that is still open: numpy/numpy#12943) |
|
We try to keep code in fixes as close to copy-paste as possible to the upstream code, so I wouldn't change anything in there. Even if an upstream fix has been merged we support several versions back so there's a ~2 year delay before we can remove things from fixes |
mne/utils/misc.py
Outdated
| @@ -149,7 +149,7 @@ def run_subprocess(command, return_code=False, verbose=None, *args, **kwargs): | |||
| break | |||
| else: | |||
| err = err.decode('utf-8') | |||
| logger.warning(err) | |||
| warn(err) | |||
There was a problem hiding this comment.
This seems to confuse the tests. I don't really see how, because the traceback is not very informative: https://github.com/mne-tools/mne-python/pull/9434/checks?check_run_id=2704866135
Except for three failures where coverage.py warns about something, it always looks like this:
<decorator-gen-2>:24: in run_subprocess
???
mne/utils/misc.py:152: in run_subprocess
warn(err)
mne/utils/_logging.py:358: in warn
warnings.warn_explicit(
E RuntimeWarning:
Do you have an idea what's going on @larsoner?
There was a problem hiding this comment.
Warnings are treated as errors in tests. Now you are emitting a warning, rather than logging with a WARNING level of verbosity, hence it raises an error. The easiest thing is probably to add an ignore line to mne/conftest.py for this "include is ignored because"
There was a problem hiding this comment.
... actually in this case I think logger.warn is arguably the correct behavior since this is basically an advanced wrapper for subprocess. I think we should restore the logger.warning for this one to mirror the logger.info that is used for stdout-level messages.
There was a problem hiding this comment.
thanks, that makes sense. Also thanks for directly pushing those commits. It all passes now.
sappelhoff
changed the title
* upstream/main: [MRG] change utils.logger.warning -> utils.warn (mne-tools#9434) FIX : rank computation from info now uses SSS proc history if only grad or mag are present (mne-tools#9435) MRG: Enable interpolation for all fNIRS types (mne-tools#9431) FIX: brain save_movie (mne-tools#9426) ENH: Add mne.export (mne-tools#9427) ENH: Test more on pre [skip circle] (mne-tools#9423) MRG, ENH: Speed up brain test (mne-tools#9422) MAINT: Update URL [ci skip] MNT: Reduce number of calls to _update (mne-tools#9407) MRG: Tutorial improvements (mne-tools#9416)
closes #9432
A simple PR to improve consistency in the code base: using
mne.utils.warn, instead of different warning calls.